From 4f31afd21b208965bae734396f6e2c6df06684c7 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 2 Mar 2018 12:42:57 -0800 Subject: [PATCH] Move most User::clearAllNotifications() logic to WatchedItemStore Change-Id: Ib1b0c40e408f6fad6fc8257c5073fa1c3c264c3a --- autoload.php | 1 + includes/DefaultSettings.php | 3 +- includes/jobqueue/jobs/ActivityUpdateJob.php | 15 +++- .../jobs/ClearWatchlistNotificationsJob.php | 79 +++++++++++++++++++ includes/user/User.php | 48 +---------- .../watcheditem/NoWriteWatchedItemStore.php | 4 + includes/watcheditem/WatchedItemStore.php | 26 +++++- .../watcheditem/WatchedItemStoreInterface.php | 13 ++- 8 files changed, 135 insertions(+), 54 deletions(-) create mode 100644 includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php diff --git a/autoload.php b/autoload.php index eb3d776634..0b0c288ffb 100644 --- a/autoload.php +++ b/autoload.php @@ -271,6 +271,7 @@ $wgAutoloadLocalClasses = [ 'CleanupUsersWithNoId' => __DIR__ . '/maintenance/cleanupUsersWithNoId.php', 'ClearInterwikiCache' => __DIR__ . '/maintenance/clearInterwikiCache.php', 'ClearUserWatchlistJob' => __DIR__ . '/includes/jobqueue/jobs/ClearUserWatchlistJob.php', + 'ClearWatchlistNotificationsJob' => __DIR__ . '/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php', 'CliInstaller' => __DIR__ . '/includes/installer/CliInstaller.php', 'CloneDatabase' => __DIR__ . '/includes/db/CloneDatabase.php', 'CodeCleanerGlobalsPass' => __DIR__ . '/maintenance/CodeCleanerGlobalsPass.inc', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 538c1b25a9..f473b3e7f7 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -7454,8 +7454,9 @@ $wgJobClasses = [ 'categoryMembershipChange' => CategoryMembershipChangeJob::class, 'clearUserWatchlist' => ClearUserWatchlistJob::class, 'cdnPurge' => CdnPurgeJob::class, - 'enqueue' => EnqueueJob::class, // local queue for multi-DC setups 'userGroupExpiry' => UserGroupExpiryJob::class, + 'clearWatchlistNotifications' => ClearWatchlistNotificationsJob::class, + 'enqueue' => EnqueueJob::class, // local queue for multi-DC setups 'null' => NullJob::class, ]; diff --git a/includes/jobqueue/jobs/ActivityUpdateJob.php b/includes/jobqueue/jobs/ActivityUpdateJob.php index da4ec2336d..8cc14e51e8 100644 --- a/includes/jobqueue/jobs/ActivityUpdateJob.php +++ b/includes/jobqueue/jobs/ActivityUpdateJob.php @@ -22,6 +22,12 @@ /** * Job for updating user activity like "last viewed" timestamps * + * Job parameters include: + * - type: one of (updateWatchlistNotification) [required] + * - userid: affected user ID [required] + * - notifTime: timestamp to set watchlist entries to [required] + * - curTime: UNIX timestamp of the event that triggered this job [required] + * * @ingroup JobQueue * @since 1.26 */ @@ -29,8 +35,10 @@ class ActivityUpdateJob extends Job { function __construct( Title $title, array $params ) { parent::__construct( 'activityUpdateJob', $title, $params ); - if ( !isset( $params['type'] ) ) { - throw new InvalidArgumentException( "Missing 'type' parameter." ); + static $required = [ 'type', 'userid', 'notifTime', 'curTime' ]; + $missing = implode( ', ', array_diff( $required, array_keys( $this->params ) ) ); + if ( $missing != '' ) { + throw new InvalidArgumentException( "Missing paramter(s) $missing" ); } $this->removeDuplicates = true; @@ -40,8 +48,7 @@ class ActivityUpdateJob extends Job { if ( $this->params['type'] === 'updateWatchlistNotification' ) { $this->updateWatchlistNotification(); } else { - throw new InvalidArgumentException( - "Invalid 'type' parameter '{$this->params['type']}'." ); + throw new InvalidArgumentException( "Invalid 'type' '{$this->params['type']}'." ); } return true; diff --git a/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php b/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php new file mode 100644 index 0000000000..94c1351a79 --- /dev/null +++ b/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php @@ -0,0 +1,79 @@ +params ) ) ); + if ( $missing != '' ) { + throw new InvalidArgumentException( "Missing paramter(s) $missing" ); + } + + $this->removeDuplicates = true; + } + + public function run() { + $services = MediaWikiServices::getInstance(); + $lbFactory = $services->getDBLoadBalancerFactory(); + $rowsPerQuery = $services->getMainConfig()->get( 'UpdateRowsPerQuery' ); + + $dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER ); + $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ ); + + $asOfTimes = array_unique( $dbw->selectFieldValues( + 'watchlist', + 'wl_notificationtimestamp', + [ 'wl_user' => $this->params['userId'], 'wl_notificationtimestamp IS NOT NULL' ], + __METHOD__, + [ 'ORDER BY' => 'wl_notificationtimestamp DESC' ] + ) ); + + foreach ( array_chunk( $asOfTimes, $rowsPerQuery ) as $asOfTimeBatch ) { + $dbw->update( + 'watchlist', + [ 'wl_notificationtimestamp' => null ], + [ + 'wl_user' => $this->params['userId'], + 'wl_notificationtimestamp' => $asOfTimeBatch, + // New notifications since the reset should not be cleared + 'wl_notificationtimestamp < ' . + $dbw->addQuotes( $dbw->timestamp( $this->params['casTime'] ) ) + ], + __METHOD__ + ); + $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket ); + } + } +} diff --git a/includes/user/User.php b/includes/user/User.php index fda49a57f8..9777148b38 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -3969,51 +3969,9 @@ class User implements IDBAccessObject, UserIdentity { return; } - $dbw = wfGetDB( DB_MASTER ); - $asOfTimes = array_unique( $dbw->selectFieldValues( - 'watchlist', - 'wl_notificationtimestamp', - [ 'wl_user' => $id, 'wl_notificationtimestamp IS NOT NULL' ], - __METHOD__, - [ 'ORDER BY' => 'wl_notificationtimestamp DESC', 'LIMIT' => 500 ] - ) ); - if ( !$asOfTimes ) { - return; - } - // Immediately update the most recent touched rows, which hopefully covers what - // the user sees on the watchlist page before pressing "mark all pages visited".... - $dbw->update( - 'watchlist', - [ 'wl_notificationtimestamp' => null ], - [ 'wl_user' => $id, 'wl_notificationtimestamp' => $asOfTimes ], - __METHOD__ - ); - // ...and finish the older ones in a post-send update with lag checks... - DeferredUpdates::addUpdate( new AutoCommitUpdate( - $dbw, - __METHOD__, - function () use ( $dbw, $id ) { - global $wgUpdateRowsPerQuery; - - $lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory(); - $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ ); - $asOfTimes = array_unique( $dbw->selectFieldValues( - 'watchlist', - 'wl_notificationtimestamp', - [ 'wl_user' => $id, 'wl_notificationtimestamp IS NOT NULL' ], - __METHOD__ - ) ); - foreach ( array_chunk( $asOfTimes, $wgUpdateRowsPerQuery ) as $asOfTimeBatch ) { - $dbw->update( - 'watchlist', - [ 'wl_notificationtimestamp' => null ], - [ 'wl_user' => $id, 'wl_notificationtimestamp' => $asOfTimeBatch ], - __METHOD__ - ); - $lbFactory->commitAndWaitForReplication( __METHOD__, $ticket ); - } - } - ) ); + $watchedItemStore = MediaWikiServices::getInstance()->getWatchedItemStore(); + $watchedItemStore->resetAllNotificationTimestampsForUser( $this ); + // We also need to clear here the "you have new message" notification for the own // user_talk page; it's cleared one page view later in WikiPage::doViewUpdates(). } diff --git a/includes/watcheditem/NoWriteWatchedItemStore.php b/includes/watcheditem/NoWriteWatchedItemStore.php index 1a0f504714..86e7be855e 100644 --- a/includes/watcheditem/NoWriteWatchedItemStore.php +++ b/includes/watcheditem/NoWriteWatchedItemStore.php @@ -122,6 +122,10 @@ class NoWriteWatchedItemStore implements WatchedItemStoreInterface { throw new DBReadOnlyError( null, 'The watchlist is currently readonly.' ); } + public function resetAllNotificationTimestampsForUser( User $user ) { + throw new DBReadOnlyError( null, 'The watchlist is currently readonly.' ); + } + public function resetNotificationTimestamp( User $user, Title $title, diff --git a/includes/watcheditem/WatchedItemStore.php b/includes/watcheditem/WatchedItemStore.php index 1b37968843..6e907deef8 100644 --- a/includes/watcheditem/WatchedItemStore.php +++ b/includes/watcheditem/WatchedItemStore.php @@ -504,7 +504,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac * @since 1.27 * @param User $user * @param LinkTarget $target - * @return bool + * @return WatchedItem|bool */ public function loadWatchedItem( User $user, LinkTarget $target ) { // Only loggedin user can have a watchlist @@ -765,12 +765,34 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return $success; } + public function resetAllNotificationTimestampsForUser( User $user ) { + // Only loggedin user can have a watchlist + if ( $user->isAnon() ) { + return; + } + + // If the page is watched by the user (or may be watched), update the timestamp + $job = new ClearWatchlistNotificationsJob( + $user->getUserPage(), + [ 'userId' => $user->getId(), 'casTime' => time() ] + ); + + // Try to run this post-send + // Calls DeferredUpdates::addCallableUpdate in normal operation + call_user_func( + $this->deferredUpdatesAddCallableUpdateCallback, + function () use ( $job ) { + $job->run(); + } + ); + } + /** * @since 1.27 * @param User $editor * @param LinkTarget $target * @param string|int $timestamp - * @return int + * @return int[] */ public function updateNotificationTimestamp( User $editor, LinkTarget $target, $timestamp ) { $dbw = $this->getConnectionRef( DB_MASTER ); diff --git a/includes/watcheditem/WatchedItemStoreInterface.php b/includes/watcheditem/WatchedItemStoreInterface.php index 133f480cdf..a450ae5347 100644 --- a/includes/watcheditem/WatchedItemStoreInterface.php +++ b/includes/watcheditem/WatchedItemStoreInterface.php @@ -209,7 +209,7 @@ interface WatchedItemStoreInterface { /** * @since 1.31 * - * @param User $user The user to set the timestamp for + * @param User $user The user to set the timestamps for * @param string|null $timestamp Set the update timestamp to this value * @param LinkTarget[] $targets List of targets to update. Default to all targets * @@ -221,6 +221,15 @@ interface WatchedItemStoreInterface { array $targets = [] ); + /** + * Reset all watchlist notificaton timestamps for a user using the job queue + * + * @since 1.31 + * + * @param User $user The user to reset the timestamps for + */ + public function resetAllNotificationTimestampsForUser( User $user ); + /** * @since 1.31 * @@ -246,7 +255,7 @@ interface WatchedItemStoreInterface { * @param int $oldid The revision id being viewed. If not given or 0, latest revision is * assumed. * - * @return bool success + * @return bool success Whether a job was enqueued */ public function resetNotificationTimestamp( User $user, Title $title, $force = '', $oldid = 0 ); -- 2.20.1